Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autogenerate spec_version and make it optional in netkans #4155

Merged
merged 1 commit into from
Aug 11, 2024

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Aug 11, 2024

Background

spec_version in a .ckan file defines the earliest version of the CKAN client capable of loading a module and installing it correctly. Any modules with a spec version later than your CKAN client version will be hidden as if they did not exist, to avoid broken installs or crashing your client.

Currently the spec version is required in a .netkan file and must be set to a safe version manually by the Metadata Wranglers. If a module uses a feature that is unavailable in its configured spec version, an inflation error is emitted and the module isn't indexed.

Motivation

Manually maintaining the spec version is tedious and error-prone. It is hard to know which features require specific versions or the versions they require, and it will only get more difficult as more versions with more features are released. Since this knowledge was added to the Netkan validators, we typically rely on inflation errors to alert us when there's a problem, which feels like unnecessarily inserting human action into a potentially fully automatable process. This is pretty off-putting for new Metadata Wranglers to have to deal with.

We can't even limit the impact of this to the CKAN team because author-maintained metanetkans and internal .ckan files also must obey the spec version rules. If the KSP-RO folks decide they want to use a brand new feature in one of their many metanetkans, either they would need to update the spec version on their own with no guardrails to let them know it's needed, or we would have to spot inflation errors from the bot and send them a pull request to update it.

Changes

  • Now after it completes all the other inflation steps, Netkan uses a new SpecVersionTransformer to analyze the inflated .ckan file and deduce the earliest compatible spec version that is safe to use, then sets it in the module. All of the existing spec version validation checks (which are still present as a safety measure) have been copied and modified to return the needed versions instead of throwing exceptions.
  • The CKAN.NetKAN.Model.Metadata class no longer throws an exception in its constructor if the input JSON object has no spec_version property
  • To make this work, several validators that depend on the spec version are moved from the pre-inflation stage to the post-inflation stage, after the autogenerated spec version has been set.
  • Similarly, transformers that previously handled merging of multiple spec versions (for internal .ckan files and metanetkans) no longer do so, because one or both might be null, and a single correct spec version will be calculated at the end anyway.

This makes spec_version optional in .netkan files (though still required in .ckan files); in effect, it has been changed from a human-maintained value to an autogenerated value. Merging these changes will cause the bot to update indexed modules to have the earliest compatible spec versions. It will also allow us to remove the now-optional-and-ignored spec version from existing netkans, the SpaceDock Adder, and the Metadata Webtool.

Historical notes

To make sure that setting earlier spec versions will be safe for existing modules, I wanted to confirm that default install stanzas did not require a specific spec version. After some digging in the git log, it looks like they were added in #127, which was included in these releases:

All of which happened before version 1 of the spec, which corresponded to:

So it is confirmed safe to omit install in any spec version.

@HebaruSan HebaruSan added Enhancement New features or functionality Netkan Issues affecting the netkan data labels Aug 11, 2024
@HebaruSan HebaruSan merged commit cab3c32 into KSP-CKAN:master Aug 11, 2024
3 checks passed
@HebaruSan HebaruSan deleted the feature/auto-spec-version branch August 11, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New features or functionality Netkan Issues affecting the netkan data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant